-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new action for bulk-modifying all tasks currently in view (addresses #240) #290
base: 2.x
Are you sure you want to change the base?
Conversation
This action is a first use case (but really just one special case) of bulk editing, which makes use of the new 'modify_multiple' operation by passing the filter expression that matches all tasks visible in the current view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking fairly good on first inspection, just a few minor requests.
Looking good, should I add something to the documentation as well? (Where?) |
I say we make it a top-ticket item at https://github.com/vit-project/vit#features
There's no great place yet to add a more detailed description. The two best candidates seem to be:
Probably the sample config is the best place, since you can offer an example of the token being used. Thoughts? |
I'm not sure it's that crucial of a feature to warrant explicit mention on the front page, maybe mentioning it in Adding it to the sample config sounds good, how would you feel about replacing vit/vit/config/config.sample.ini Lines 194 to 196 in 1d59db8
with # For capital letter keybindings, use the letter directly:
# m = {ACTION_TASK_MODIFY}
# M = {ACTION_TASK_MODIFY_ALL} ? |
Yes, see scripts/generate-changelog-entries.sh -- but we won't do that until we're ready for a release. If this feature was 'bulk edit', then it would definitely warrant mention in the features. The currrent implementation isn't quite there yet. It's probably best to wait.
Just add it, we can have multiple examples there. Also, because it's a registered action, it appears in the output of I think that's enough for now. |
To be consistent with taskwarrior terminology, bulk modification (not editing, which is completely interactive) is simply modification with a filter expression that matches more than one task. Subject to the #240 implies a second feature, which I would call manually selecting tasks (for the purpose of calling bulk modification on them), but that is IMHO functionality that combines two features that should be treated separately (because each have their own uses): the internal bulk modification operation on one hand, and a new UI feature like #291 on the other. |
Just added a binding (including a comment regarding the I also just realised from writing the above that my choice of |
Yep, sounds good. Thanks for for refresher on bulk modifications, and it does convince me more that 'bulk modifications' should be in the main feature list -- toss it in there! We can adjust/enhance it if/when the other multi-task modification feature lands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're very, very close now on the code review!
2253adf
to
9d33758
Compare
OK, ran some quick tests, we could be there. I have two questions:
|
|
|
Here we go, a new user-facing action
{ACTION_TASK_MODIFY_ALL}
which bulk-modifies all tasks currently visible in vit by means of a new'modify_multiple'
operation. This new internal operation is not limited to this one use case, but can also be used to bulk-modify sets of tasks that are selected by other means (e.g. manually by the user, as was discussed in #240).Note that I have gone for the option of adding a new operation
'modify_multiple'
just because I didn't want to touch the original'modify'
operation -- except for some extra safety checks whether the number of affected tasks is still the same and a different status bar message at the end, these two operations execute the same command. So instead of having two operations with different names it would also be possible to stick to just one'modify'
operation and distinguish the use cases internally based on their arguments (in particular whether auuid
is passed or a pair oftarget
/ntasks
).I realised some of the filter-concatenation functionality of my earlier PR #289 was already implemented in
task.py
, so I've made use of the existing methods instead of adding my own.